[sql lab] Fix setting async query status to timed_out #7429
Conversation
graceguo-supercat
force-pushed
the
gg-FixQueryTimedoutStatus
branch
from
7d4002c to
5883c22
Compare
Codecov Report
@@ Coverage Diff @@
## master #7429 +/- ##
==========================================
- Coverage 65.26% 65.25% -0.01%
==========================================
Files 430 430
Lines 21077 21080 +3
Branches 2338 2338
==========================================
Hits 13756 13756
- Misses 7205 7208 +3
Partials 116 116
Continue to review full report at Codecov.
|
| Query.client_id in queries_to_timeout, | ||
| ), | ||
| ).values(state=QueryStatus.TIMED_OUT) | ||
| for q in sql_queries: |
There was a problem hiding this comment.
How many SQL queries are there? It may be more efficient to have the database (like before) handle this logic.
The issue here is that update(...) returns an expression which then needs to be executed and committed.
|
Hi @mistercrunch I have concern for this PR. We have 1.5M rows in query table since last June, the number is growing very fast. There may have over hundred ppl polling (and update) query status at the same time, for every second. At the same time, new queries are creating, and it also updates same table. My concern is will this PR (and #4833) make query table too busy to handle, and finally cause table lock issue? |
|
concern on the amount of polling requests, i think #7498 might be better. |
CATEGORY
Choose one
SUMMARY
In #4833, we tried to introduce a feature that setting old async query status to be
timed_out, when the query status stuck at pending or running without updates after 6 hours. This status will help front-end (browser) not fetching very old query status. The bug i found is backend didn't commit transaction, so there is no records in query table has statustimed_out(but keeps in running status for very very long time).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
When user's query has no updates after 6 hours, front-end should show this error message:
TEST PLAN
send async query, then check query table. We should see old queries status that were
runningorpending, should updated to betimed_out.ADDITIONAL INFORMATION
REVIEWERS
@john-bodley @michellethomas @timifasubaa @mistercrunch